[Draft] Test gemini code review: Feature/imprintfix by NickSzapiro-NOAA · Pull Request #14 · NickSzapiro-NOAA/ufs-weather-model

NickSzapiro-NOAA · 2025-04-30T14:50:27Z

Commit Queue Requirements:

Fill out all sections of this template.
All sub component pull requests have been reviewed by their code managers.
Run the full Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
Commit 'test_changes.list' from previous step

Description:

Commit Message:

* UFSWM - 
  * AQM - 
  * CDEPS - 
  * CICE - 
  * CMEPS - 
  * CMakeModules - 
  * FV3 - 
    * ccpp-physics - 
    * atmos_cubed_sphere - 
  * GOCART - 
  * HYCOM - 
  * MOM6 - 
  * NOAHMP - 
  * WW3 - 
  * fire_behavior
  * stochastic_physics -

Priority:

Critical Bugfix: Reason
High: Reason
Normal

Git Tracking

UFSWM:

Closes #
None

Sub component Pull Requests:

AQM:
CDEPS:
CICE:
CMEPS:
CMakeModules:
FV3:
- ccpp-physics:
- atmos_cubed_sphere:
GOCART:
HYCOM:
MOM6:
NOAHMP:
WW3:
fire_behavior:
stochastic_physics:
None

UFSWM Blocking Dependencies:

Blocked by #
None

Changes

Regression Test Changes (Please commit test_changes.list):

PR Adds New Tests/Baselines.
PR Updates/Changes Baselines.
No Baseline Changes.

Input data Changes:

None.
New input data.
Updated input data.

Library Changes/Upgrades:

Required
- Library names w/versions:
- Git Stack Issue (JCSDA/spack-stack#)
No Updates

Testing Log:

This reverts commit 8e3253c.

* with ESMF8.6, stripes bug causes differences in mediator restart files, but they do not change results, all other files are b4b. Remove them until ESMF8.8 is adopted

Your branch is up to date with 'origin/feature/imprintfix'.

…mmunity#2661) * UFSWM - NSSL-MP scheme updates * FV3 - NSSL-MP scheme updates * ccpp-physics - NSSL-MP scheme updates + scientific documentation updates for SRW v3.0.0 * stochastic_physics - enable standalone unit tests and Jenkins pipeline git integration * MOM6 hash update 25ed

…nfoBroadcast in fv3 cap run phase2 (was ufs-community#2721) (ufs-community#2715) * UFSWM - Remove multigrid logical from scripts * UFSWM - Add switch to disable InfoBroadcast in fv3 cap run phase2 * FV3 - Add switch to disable InfoBroadcast in fv3 cap run phase2 * WW3 - Refactor mesh cap to move call to w3init into Advertise phase --------- Co-authored-by: Dusan Jovic <dusan.jovic@noaa.gov>

NickSzapiro-NOAA · 2025-05-02T19:49:39Z

/gemini review

gemini-code-assist

Code Review

This pull request includes several updates to submodules, configuration files, and test logs. The changes seem to be related to bug fixes and baseline updates. However, it's crucial to verify the correctness and impact of these changes, especially the removal of the WW3_MULTIGRID variable and the updates to submodule commits and test dates.

Summary of Findings

Submodule Updates: The pull request updates several submodules. It's important to ensure that these updates are intentional and don't introduce unintended changes or regressions. Verify that the new commits in each submodule are the desired ones and that they align with the overall goals of the pull request.
Test Date Update: The baseline date in tests/bl_date.conf has been updated. Confirm that this change is intentional and reflects the correct date for the tests.
Removal of WW3_MULTIGRID: The variable WW3_MULTIGRID has been removed from tests/default_vars.sh and the export_cmeps function. Verify that this removal doesn't negatively impact any functionality or tests that rely on this variable.
Inconsistent Working Directory in Logs: The working directory in the log files is being updated from opnReqTest_285605 to opnReqTest_1961488. It's important to verify that this change is correct and doesn't point to an incorrect or non-existent directory.

Merge Readiness

While the pull request seems to address some bug fixes and updates, the presence of medium and high severity issues warrants further investigation. Specifically, the removal of WW3_MULTIGRID and the potential inconsistency in working directories in log files need to be clarified. I recommend that the pull request not be merged until these issues are addressed and the changes are thoroughly tested. I am unable to approve this pull request, and other reviewers should also examine the code before a decision is made to merge it.

gemini-code-assist · 2025-05-02T19:50:11Z

 export WW3_IC5='F'
 # ATMW
-export WW3_MULTIGRID=true
 export WW3_MODDEF=mod_def.glo_1deg


The variable WW3_MULTIGRID is being removed. What is the justification for this removal? Does this variable no longer serve a purpose, or is its functionality being replaced by something else? Please provide context or documentation for this change.

gemini-code-assist · 2025-05-02T19:50:11Z


 export_cpl ()


The variable WW3_MULTIGRID is being removed from this function as well. Please confirm that this removal is intentional and doesn't break any existing functionality. What is the impact of removing this variable on the CMEPS component?

gemini-code-assist · 2025-05-02T19:50:11Z


 baseline dir = /scratch1/NCEPDEV/stmp4/role.epic/FV3_OPNREQ_TEST/OPNREQ_TEST/control_p8_bit_base_gnu
-working dir  = /scratch1/NCEPDEV/stmp2/role.epic/FV3_OPNREQ_TEST/opnReqTest_285605/bit_base_bit_base
+working dir  = /scratch1/NCEPDEV/stmp2/role.epic/FV3_OPNREQ_TEST/opnReqTest_1961488/bit_base_bit_base


The working directory is being updated from opnReqTest_285605 to opnReqTest_1961488. Is this change correct? Does the new directory exist and contain the expected test results? Please verify that this change doesn't point to an incorrect or non-existent directory.

gemini-code-assist · 2025-05-02T19:50:11Z


 baseline dir = /scratch1/NCEPDEV/stmp4/role.epic/FV3_OPNREQ_TEST/OPNREQ_TEST/cpld_control_c96_noaero_p8_dbg_base_gnu
-working dir  = /scratch1/NCEPDEV/stmp2/role.epic/FV3_OPNREQ_TEST/opnReqTest_3765011/dbg_base_dbg_base
+working dir  = /scratch1/NCEPDEV/stmp2/role.epic/FV3_OPNREQ_TEST/opnReqTest_1430717/dbg_base_dbg_base


The working directory is being updated from opnReqTest_3765011 to opnReqTest_1430717. Is this change correct? Does the new directory exist and contain the expected test results? Please verify that this change doesn't point to an incorrect or non-existent directory.

gemini-code-assist · 2025-05-02T19:50:11Z


 baseline dir = /scratch1/NCEPDEV/stmp4/role.epic/FV3_OPNREQ_TEST/OPNREQ_TEST/regional_control_bit_base_gnu
-working dir  = /scratch1/NCEPDEV/stmp2/role.epic/FV3_OPNREQ_TEST/opnReqTest_2150585/bit_base_bit_base
+working dir  = /scratch1/NCEPDEV/stmp2/role.epic/FV3_OPNREQ_TEST/opnReqTest_194426/bit_base_bit_base


The working directory is being updated from opnReqTest_2150585 to opnReqTest_194426. Is this change correct? Does the new directory exist and contain the expected test results? Please verify that this change doesn't point to an incorrect or non-existent directory.

on-behalf-of @ufs-community <ecc.platform@noaa.gov>

NickSzapiro-NOAA · 2025-05-05T13:53:34Z

/gemini review

gemini-code-assist

Code Review

The pull request includes several updates to submodule commits, test configurations, and regression test logs. The changes seem to be focused on updating the system to a new baseline and incorporating vector remapping. However, there are a few inconsistencies and a test failure that need to be addressed before merging.

Summary of Findings

Missing GaeaC5 Test: The GaeaC5 test is removed from the pull request template, but there is no clear reason provided for its removal. It's important to understand why this test is no longer needed or applicable.
Inconsistent vector remapping: The MAPUV3D variable is set to true in export_cpl and false in export_datm_cdeps. This inconsistency should be reviewed to ensure the correct value is used in each context.
Failed Test: The RegressionTests_acorn.log indicates that the test atmwav_control_noaero_p8_intel failed due to an inability to complete the comparison. This failure needs to be investigated and resolved.

Merge Readiness

Due to the atmwav_control_noaero_p8_intel test failure and the inconsistent MAPUV3D variable settings, I recommend that this pull request not be merged until these issues are investigated and resolved. I am unable to approve this pull request, and other reviewers should examine this code before merging.

gemini-code-assist · 2025-05-05T13:54:11Z

+  # vector remapping
+  export MAPUV3D=false


MAPUV3D is set to false here. This is different from the export_cpl function. Please confirm that this is the correct value for the datm_cdeps configuration.

gemini-code-assist · 2025-05-05T13:54:11Z

+PASS -- TEST 'control_p8_atmlnd_debug_intel' [11:35, 05:30](1920 MB)
+
+PASS -- COMPILE 'atmw_intel' [10:25, 08:44] ( 3 remarks )
+FAILED: UNABLE TO COMPLETE COMPARISON -- TEST 'atmwav_control_noaero_p8_intel' [, ]( MB)


The test atmwav_control_noaero_p8_intel failed. Please investigate the cause of this failure and ensure it is resolved before merging.

gemini-code-assist · 2025-05-05T13:54:11Z

      pio_rearranger = @[CMEPS_PIO_REARR]
      pio_root = @[CMEPS_PIO_ROOT]
      ocean_albedo_limit = @[ocean_albedo_limit]
+      mapuv_with_cart3d = @[MAPUV3D]


The mapuv_with_cart3d variable is being added here. Please confirm that this is the correct value for all cases where MED_attributes is used.

gemini-code-assist · 2025-05-05T13:54:11Z

      pio_rearranger = @[CMEPS_PIO_REARR]
      pio_root = @[CMEPS_PIO_ROOT]
      ocean_albedo_limit = @[ocean_albedo_limit]
+      mapuv_with_cart3d = @[MAPUV3D]


The mapuv_with_cart3d variable is being added here. Please confirm that this is the correct value for all cases where MED_attributes is used.

gemini-code-assist · 2025-05-05T13:54:12Z

    cp    @[INPUTDATA_ROOT_WW3]/@[WW3_MODDEF] mod_def.ww3
    cp    @[INPUTDATA_ROOT_WW3]/@[MESH_WAV] .
-  fi
 fi


The WW3_MULTIGRID check is being removed here. Was this intentional? If so, please provide a reason for its removal in the pull request description.

gemini-code-assist · 2025-05-05T13:54:14Z

      pio_rearranger = @[CMEPS_PIO_REARR]
      pio_root = @[CMEPS_PIO_ROOT]
      ocean_albedo_limit = @[ocean_albedo_limit]
+      mapuv_with_cart3d = @[MAPUV3D]


The mapuv_with_cart3d variable is being added here. Please confirm that this is the correct value for all cases where MED_attributes is used.

gemini-code-assist · 2025-05-05T13:54:15Z

      pio_rearranger = @[CMEPS_PIO_REARR]
      pio_root = @[CMEPS_PIO_ROOT]
      ocean_albedo_limit = @[ocean_albedo_limit]
+      mapuv_with_cart3d = @[MAPUV3D]


The mapuv_with_cart3d variable is being added here. Please confirm that this is the correct value for all cases where MED_attributes is used.

gemini-code-assist · 2025-05-05T13:54:15Z

 export RUNTYPE=startup
 export CMEPS_RESTART_DIR=./RESTART/
 export cap_dbug_flag=0
+export MAPUV3D=false


Adding MAPUV3D=false here. Please confirm that this is the correct value for this test case.

gemini-code-assist · 2025-05-05T13:54:15Z

                   RESTART/20210323.060000.MOM.res.nc \
                   RESTART/iced.2021-03-23-21600.nc \
-                   RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc \
                   ufs.cpld.ww3.r.2021-03-23-21600.nc \


The file RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc is being removed from the list. Was this intentional? If so, please provide a reason for its removal in the pull request description.

gemini-code-assist · 2025-05-05T13:54:15Z

                   RESTART/20210323.060000.MOM.res.nc \
                   RESTART/iced.2021-03-23-21600.nc \
-                   RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc \
                   ufs.cpld.ww3.r.2021-03-23-21600.nc \


The file RESTART/ufs.cpld.cpl.r.2021-03-23-21600.nc is being removed from the list. Was this intentional? If so, please provide a reason for its removal in the pull request description.

DeniseWorthen added 30 commits September 24, 2024 11:47

point to new CICE branch in my fork

8e3253c

Revert "point to new CICE branch in my fork"

cc3bbe9

This reverts commit 8e3253c.

Merge branch 'ufs-community:develop' into develop

c242681

Merge branch 'ufs-community:develop' into develop

db31e9f

Merge branch 'ufs-community:develop' into develop

f0a358b

Merge branch 'ufs-community:develop' into develop

a4444e7

Merge branch 'ufs-community:develop' into develop

1f53677

Merge branch 'ufs-community:develop' into develop

75f4e7d

Merge branch 'ufs-community:develop' into develop

fbe0495

Merge branch 'ufs-community:develop' into develop

ac0172e

Merge remote-tracking branch 'upstream/develop' into develop

5dbecc8

Merge branch 'ufs-community:develop' into develop

bcc1d7d

Merge branch 'ufs-community:develop' into develop

88b8533

Merge branch 'ufs-community:develop' into develop

30b6967

Merge branch 'ufs-community:develop' into develop

18f833d

Merge branch 'ufs-community:develop' into develop

53b9c90

Merge remote-tracking branch 'upstream/develop' into HEAD

3b131d5

Merge branch 'ufs-community:develop' into develop

a29762a

Merge branch 'ufs-community:develop' into develop

32b8157

Merge branch 'ufs-community:develop' into develop

2d6b385

Merge branch 'ufs-community:develop' into develop

f847448

Merge branch 'ufs-community:develop' into develop

a87fd23

Merge branch 'ufs-community:develop' into develop

7157552

Merge branch 'ufs-community:develop' into develop

78a03f0

Merge branch 'ufs-community:develop' into develop

11fb929

Merge branch 'ufs-community:develop' into develop

cfc00b3

Merge branch 'ufs-community:develop' into develop

67323d9

Merge branch 'ufs-community:develop' into develop

6dd5e06

Merge branch 'ufs-community:develop' into develop

9d471e6

Merge branch 'ufs-community:develop' into develop

28a806a

DeniseWorthen and others added 12 commits April 30, 2025 13:46

Merge branch 'develop' into feature/imprintfix

14de7f1

remove mediator restart files to resolve test failures

7ca7625

* with ESMF8.6, stripes bug causes differences in mediator restart files, but they do not change results, all other files are b4b. Remove them until ESMF8.8 is adopted

All tests pass against staged baseline on Derecho

599c683

Your branch is up to date with 'origin/feature/imprintfix'.

Merge branch 'ufs-community:develop' into develop

cc11ea3

re-verified test_changes.list is correct on Hera

1d4984b

All tests pass against a staged baseline on C6

7ccc5f6

Merge branch 'develop' into feature/imprintfix

e265f34

Merge branch 'ufs-community:develop' into develop

ff5fc74

Merge branch 'develop' into feature/imprintfix

4b3a3b9

Update bl_date.conf

ccbcfdd

NickSzapiro-NOAA changed the title ~~Test PR: Feature/imprintfix~~ [Draft] Test gemini code review: Feature/imprintfix May 2, 2025

gemini-code-assist Bot suggested changes May 2, 2025

View reviewed changes

DeniseWorthen and others added 12 commits May 3, 2025 05:56

fix bad merge

4d4780a

fix stray edit

a7e1789

WCOSS2 RT Log: Passed

8c77ce4

ACorn RT Log: Passed

216b1e5

add hera RT log: passed

c0f8c81

add derecho RT log: passed

727a4b1

add orion RT log : passed

325a254

add gaeac6 RT log: passed

fb30d2c

ORT Jobs Completed.

e5ff109

on-behalf-of @ufs-community <ecc.platform@noaa.gov>

add hercules RT log: passed

7fe9e91

update git template: deprecation gaeac5

def0114

update CMEPS and revert gitmodules

8492c30

gemini-code-assist Bot suggested changes May 5, 2025

View reviewed changes

DeniseWorthen deleted the feature/imprintfix branch February 19, 2026 19:16

Conversation

NickSzapiro-NOAA commented Apr 30, 2025

Commit Queue Requirements:

Description:

Commit Message:

Priority:

Git Tracking

UFSWM:

Sub component Pull Requests:

UFSWM Blocking Dependencies:

Changes

Regression Test Changes (Please commit test_changes.list):

Input data Changes:

Library Changes/Upgrades:

Testing Log:

Uh oh!

NickSzapiro-NOAA commented May 2, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist Bot May 2, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2025

Choose a reason for hiding this comment

Uh oh!

NickSzapiro-NOAA commented May 5, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels